Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Babel for react-dev-utils #2755

Closed

Conversation

viankakrisna
Copy link
Contributor

In response to #2638 (comment)

@viankakrisna
Copy link
Contributor Author

I'm lost on how to make lerna works when there's a build dependency between packages -.-' will sleep on it...

@viankakrisna
Copy link
Contributor Author

Ready for review 👍

.eslintignore Outdated
@@ -3,3 +3,22 @@ build
my-app*
packages/react-scripts/template
packages/react-scripts/fixtures
packages/react-dev-utils/FileSizeReporter.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unfortunate. Can we just ignore every file in the root folder?

@@ -0,0 +1,19 @@
/FileSizeReporter.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I'd prefer we just ignore all JS files in the root folder.

@viankakrisna
Copy link
Contributor Author

I forgot we can do wildcard for ignoring files 😄

Copy link
Contributor

@gaearon gaearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gaearon gaearon added this to the 1.0.11 milestone Jul 12, 2017
@viankakrisna
Copy link
Contributor Author

viankakrisna commented Jul 12, 2017

Somehow, Travis is hanging on node 8's kitchensink. Maybe need a restart?

@viankakrisna
Copy link
Contributor Author

@gaearon rebased, anything more to get this merged?

@@ -3,3 +3,4 @@ build
my-app*
packages/react-scripts/template
packages/react-scripts/fixtures
packages/react-dev-utils/*.js
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No newline here. Is it intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

screen shot 2017-08-23 at 8 51 37 pm 1
have it here, I think GitHub truncate it?

@@ -0,0 +1 @@
/*.js
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No newline here. Is it intended? [2]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

screen shot 2017-08-23 at 8 54 10 pm
same as above

@Timer Timer modified the milestones: 1.0.x, 1.0.11 Aug 9, 2017
@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch 3 times, most recently from 972cddb to 7c3875e Compare September 10, 2017 09:03
@viankakrisna
Copy link
Contributor Author

screen shot 2017-09-11 at 1 08 32 am

seems like related to http://codetunnel.io/npm-5-changes-to-npm-link/

@Timer
Copy link
Contributor

Timer commented Sep 10, 2017

Do we need to switch away from npm@5 for CI?

@viankakrisna
Copy link
Contributor Author

need to find out why exactly it's green on #3104 and #3026 but not here

@viankakrisna viankakrisna force-pushed the react-dev-utils/babel branch 2 times, most recently from ca4de75 to 1462aa1 Compare November 12, 2017 16:41
@viankakrisna
Copy link
Contributor Author

@Timer @gaearon can we get this in?

remove build artifacts

move babel-cli babel-preset-react-app and cross-env to devDependencies

remove unused deps
@andriy-f
Copy link

It would be nice to merge this, it holds several other important issues

@gaearon
Copy link
Contributor

gaearon commented Jan 13, 2018

@andriy-f which issues?

@gaearon
Copy link
Contributor

gaearon commented Jan 13, 2018

I’m not 100% convinced we need to increase complexity here because we can tell users Node 8 is required, figure out our node_modules compilation story, and then in both cases ES6 will work fine. So maybe we should focus on that instead.

@viankakrisna
Copy link
Contributor Author

Yea, the idea is for this PR is that we want to extract service worker code to react-dev-utils, but I see what you mean.

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants